Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #1179 Deprecated SPDX licenses #1246

Closed
wants to merge 2 commits into from
Closed

Fix #1179 Deprecated SPDX licenses #1246

wants to merge 2 commits into from

Conversation

vargenau
Copy link
Contributor

@vargenau vargenau commented Oct 5, 2022

Fix all SPDX deprecated licenses including GFDL*, BSD-2-Clause-NetBSD listed on https://spdx.org/licenses/#Deprecated%20License%20Identifiers

Signed-off-by: Marc-Etienne Vargenau marc-etienne.vargenau@nokia.com

Fix all SPDX deprecated licenses including GFDL*, BSD-2-Clause-NetBSD
listed on https://spdx.org/licenses/#Deprecated%20License%20Identifiers

Signed-off-by: Marc-Etienne Vargenau <marc-etienne.vargenau@nokia.com>
Copy link
Contributor

@kzantow kzantow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @vargenau -- thank you for the contribution. I've left some specific questions, but what is the motivation for getting rid of the deprecated identifiers? These are still valid until they have been removed, right?

There is a problem, however: we generate the license list using make generate-license-list which pulls all the license identifiers from https://spdx.org/licenses/licenses.json and it will just overwrite the changes you've made here. If we were to make some changes, it would need to be done in the generate_license_list.go or similar.

Comment on lines -112 to +113
"bsd-2-clause-freebsd": "BSD-2-Clause-FreeBSD",
"bsd-2-clause-netbsd": "BSD-2-Clause-NetBSD",
"bsd-2-clause-freebsd": "BSD-2-Clause-Views",
"bsd-2-clause-netbsd": "BSD-2-Clause",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should be changing the semantics of freebsd and netbsd in this way and below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we should.

This is explained at:

Using BSD-2-Clause-FreeBSD or BSD-2-Clause-NetBSD is invalid SPDX and is reported as such by https://tools.spdx.org/app/validate/

Comment on lines -173 to +175
"bzip2-1": "bzip2-1.0.5",
"bzip2-1.0": "bzip2-1.0.5",
"bzip2-1.0.5": "bzip2-1.0.5",
"bzip2-1": "bzip2-1.0.6",
"bzip2-1.0": "bzip2-1.0.6",
"bzip2-1.0.5": "bzip2-1.0.6",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should be relabeling these licenses. If something is explicitly bzip-1.0.5, we probably shouldn't say it is 1.0.6

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines -546 to +550
"gpl-2-with-autoconf-exception": "GPL-2.0-with-autoconf-exception",
"gpl-2-with-bison-exception": "GPL-2.0-with-bison-exception",
"gpl-2-with-classpath-exception": "GPL-2.0-with-classpath-exception",
"gpl-2-with-font-exception": "GPL-2.0-with-font-exception",
"gpl-2-with-gcc-exception": "GPL-2.0-with-GCC-exception",
"gpl-2-with-autoconf-exception": "GPL-2.0-only WITH Autoconf-exception-2.0",
"gpl-2-with-bison-exception": "GPL-2.0-only WITH Bison-exception-2.2",
"gpl-2-with-classpath-exception": "GPL-2.0-only WITH Classpath-exception-2.0",
"gpl-2-with-font-exception": "GPL-2.0-only WITH Font-exception-2.0",
"gpl-2-with-gcc-exception": "GPL-2.0-only WITH GCC-exception-2.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a useful change but where did the versions come from for the exception clauses (2.0 and 2.2)? Are these necessarily the same as the deprecated identifiers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"gfdl-1": "GFDL-1.1",
"gfdl-1": "GFDL-1.1-only",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does adding -only here change the semantics? This seems like a reasonable thing to assume, but it would be good to be sure. Same question for all the other -only additions above and below.

"lgpl-2.1+": "LGPL-2.1+",
"lgpl-2.1+": "LGPL-2.1-or-later",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+ is a valid SPDX license element as of according to the license list document, as of version 2.0, we probably don't need to change these

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes "+" is still valid, but LGPL-2.1+ is explicitly deprecated and leads to invalid SPDX.

See: https://spdx.org/licenses/LGPL-2.1+.html

"standardml-nj": "StandardML-NJ",
"standardml-nj": "SMLNJ",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already covered in the smlnj case, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Signed-off-by: Marc-Etienne Vargenau <marc-etienne.vargenau@nokia.com>
@vargenau
Copy link
Contributor Author

vargenau commented Oct 5, 2022

Hi @vargenau -- thank you for the contribution. I've left some specific questions, but what is the motivation for getting rid of the deprecated identifiers? These are still valid until they have been removed, right?

There is a problem, however: we generate the license list using make generate-license-list which pulls all the license identifiers from https://spdx.org/licenses/licenses.json and it will just overwrite the changes you've made here. If we were to make some changes, it would need to be done in the generate_license_list.go or similar.

Yes, I had seen the comment

// Code generated by go generate; DO NOT EDIT.

but did not see how to modify generate_license_list.go to achieve the result.

For the motivation of the changes, deprecated identifiers leads to invalid SPDX code, as reported by https://tools.spdx.org/app/validate/

@vargenau vargenau requested a review from kzantow October 6, 2022 17:33
@spiffcs
Copy link
Contributor

spiffcs commented Oct 6, 2022

This suggestion from Dan seems to point to the correct place where we could start identifying the map from deprecated licenses to their newer versions: #950 (comment)

I think there is a way where we can update generate_license_list.go to:

  1. see if a license is deprecated
  2. if so check the fields between its deprecated object and the newer object it points to
  3. if those fields match, update the map programmatically so that we can auto-generate this file rather than manually curate it

@vargenau
Copy link
Contributor Author

vargenau commented Oct 7, 2022

Thank you for your comments.

Who will implement this? I am not sure to understand enough generate_license_list.go to be able to do it myself, but I can help.

It is very important that we generate valid SPDX.

@kzantow
Copy link
Contributor

kzantow commented Oct 7, 2022

The licenses in question are deprecated, but they should still be valid SPDX licenses, shouldn't they?

@vargenau
Copy link
Contributor Author

vargenau commented Oct 7, 2022

The licenses in question are deprecated, but they should still be valid SPDX licenses, shouldn't they?

From the standard:

When a license identifier is "deprecated" on the SPDX License List, it effectively means that there is an updated license identifier and the deprecated license identifier, while remaining valid, should no longer be used.

The SPDX validator https://tools.spdx.org/app/validate/ gives a warning.

So technically, it is valid, but it should not be used, so it still think it would be beneficial to have it fixed.

@kzantow
Copy link
Contributor

kzantow commented Oct 7, 2022

Keep in mind we aren't generating these license strings, these are identified directly from packages we've analyzed. So, if the package is using a license that is deprecated in SPDX, the package itself should be updated. Syft shouldn't change the semantics of what package it finds, generally speaking. The thought here is, if SPDX data tells us a deprecated license is otherwise exactly the same as a non-deprecated license, we could probably do that translation. That is what Dan's comment was about. We can look at getting this implemented at some point, but it doesn't seem to be something to prioritize, as again, these license are, in fact, still valid in SPDX.

And as noted before, this entire file is generated from all the valid licenses based on the latest SPDX license list, if one is no longer deprecated, but removed, it will not be in this list and it will not show up in Syft's translation file here.

@vargenau
Copy link
Contributor Author

Yes, the replacement of the deprecated license is exactly the same as the deprecated license.

I had first created #950 that has been implemented in #1009.

This replaces e.g . GPL-2.0 by GPL-2.0-only and GPL-2.0+ by GPL-2.0-and-later.

Why not doing the same with GFDL family, LGPL family, etc?

We are now in the middle of the river. #1179 is about the complete the job with all deprecated licenses.

@vargenau vargenau closed this by deleting the head repository Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants